Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Annotate intersecting roads and maneuver points along the current route #2928

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

avi-c
Copy link
Contributor

@avi-c avi-c commented Apr 13, 2021

Description

Adds a UI feature to add visible callout annotations for upcoming intersecting roads, and for annotating the next maneuver point.

Implemented using the Nav SDK v2.0 Electronic Horizon feature.

Screenshots or Gifs

Simulator Screen Shot - iPhone 11 - 2021-04-13 at 12 06 37
Simulator Screen Shot - iPhone 11 - 2021-04-13 at 12 03 34

@avi-c avi-c requested review from MaximAlien, chezzdev and 1ec5 April 13, 2021 19:12
@avi-c
Copy link
Contributor Author

avi-c commented Apr 13, 2021

@chezzdev - Please review my use of the new electronic horizon API. Also, please note that I added the RoadGraph to the set of data sent by the EHorizon notifications since that information is needed in order to lookup road network information for the edges so we can display relevant info.

@avi-c avi-c force-pushed the ac-IntersectionAnnotationFeature branch from 2d7b635 to 7acf938 Compare April 13, 2021 19:55
@avi-c
Copy link
Contributor Author

avi-c commented Apr 20, 2021

Exposed the font list for the intersection annotations style layer as a UIAppearance proxy of a list of font name strings. Allows for developer to pick fonts in order to support the geographies and languages that they need. Expanded the default set to include DIN Pro Medium as the primary font with Noto Sans CJK JP Medium and Arial Unicode MS Regular set as fallbacks.

@S2Ler
Copy link
Contributor

S2Ler commented Jun 24, 2021

Would love to see tests for the new functionality.

import MapboxCoreNavigation

extension ElectronicHorizon.Edge {
var mpp: [ElectronicHorizon.Edge]? {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mpp isn't readable to me.

@MaximAlien MaximAlien added this to the v2.2 milestone Nov 29, 2021
@MaximAlien MaximAlien added the UI Work related to visual components, Android Auto, Camera, 3D, voice, etc. label Nov 29, 2021
@ShanMa1991 ShanMa1991 modified the milestones: v2.2, v2.3 Jan 20, 2022
@MaximAlien MaximAlien self-assigned this Feb 23, 2022
@MaximAlien MaximAlien modified the milestones: v2.3, v2.4 Feb 23, 2022
@MaximAlien MaximAlien modified the milestones: v2.4, v2.5 Apr 13, 2022
@1ec5
Copy link
Contributor

1ec5 commented May 17, 2022

This may be blocked by a regression in the map SDK: mapbox/mapbox-maps-ios#381. Need to visually confirm that the tails still point to the route line and that the bubble can stretch horizontally without part of the tail stretching.

@1ec5 1ec5 removed this from the v2.5 milestone May 17, 2022
@1ec5 1ec5 force-pushed the ac-IntersectionAnnotationFeature branch from e6175ac to 1ef5c18 Compare May 17, 2022 15:51
@1ec5
Copy link
Contributor

1ec5 commented May 17, 2022

1ef5c18 has been rebased to squash some intermediate commits.

@1ec5 1ec5 self-assigned this May 17, 2022
Comment on lines +1206 to +2143
let stretchX = [ImageStretches(first: Float(20), second: Float(30)), ImageStretches(first: Float(90), second: Float(100))]
let stretchY = [ImageStretches(first: Float(26), second: Float(32))]
let imageContent = ImageContent(left: 20, top: 26, right: 100, bottom: 33)
Copy link
Contributor

@1ec5 1ec5 May 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that mapbox/mapbox-maps-ios#1269 has landed, we should define cap insets in the asset catalog instead of in code, where they’re more fragile.

{
"images" : [
{
"filename" : "AnnotationCentered.png",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These assets should be stored as scalable PDFs. #2734/#2873 introduced more stylish RouteInfoAnnotationLeftHanded and RouteInfoAnnotationRightHanded assets, but there’s no corresponding RouteInfoAnnotationCentered asset.

with: "AnnotationCentered",
stretchX: stretchX,
stretchY: stretchY,
scale: 2.0,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This scale parameter went away in mapbox/mapbox-maps-ios#321. It looks like these assets need to be scaled down by a factor of 2.

avi-c added 2 commits May 17, 2022 12:24
…rsection annotation labels. Set defaults to include a couple of fallbacks that should catch a larger number of glyphs than before.
@1ec5 1ec5 force-pushed the ac-IntersectionAnnotationFeature branch from 1ef5c18 to c02539f Compare May 17, 2022 19:30
@1ec5 1ec5 changed the base branch from release-v2.0 to main May 17, 2022 19:30
@1ec5
Copy link
Contributor

1ec5 commented May 17, 2022

This may be blocked by a regression in the map SDK: mapbox/mapbox-maps-ios#381. Need to visually confirm that the tails still point to the route line and that the bubble can stretch horizontally without part of the tail stretching.

The image stretches correctly, but the anchors are way off:

Harwick Bittern

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
UI Work related to visual components, Android Auto, Camera, 3D, voice, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants